Add UsageDetails dictionary by jarryd999 · Pull Request #69 · whatwg/storage

jarryd999 · 2019-04-22T21:23:24Z

This change adds a new dictionary to the result of StorageManager.estimate() that breaks down usage by storage system.

I've opened a related TAG review here: w3ctag/design-reviews#365

Preview | Diff

storage.bs

domenic

This overall architecture makes sense to me now (although @annevk should check, as the editor). What remains are editorial issues.

storage.bs

jarryd999 · 2019-06-04T17:32:19Z

@domenic Comments have been addressed, PTAL. If you have no further comments, then thank you for the review!

domenic

LGTM, but still needs editor approval. (And web platform tests!)

inexorabletash · 2019-06-10T19:21:14Z

@jarryd999 added "tentative" tests already in https://github.com/web-platform-tests/wpt/tree/master/storage (estimate-usage-details...)

annevk

Thanks for writing this up and thanks for your patience!

At a high-level I'm wondering about these things:

Are other implementers on board with exposing this?
Are there bugs filed against those implementations?
Is there a PR to make the tests stop being tentative that we can land together with this assuming all is in order? (There's a couple minor things I wonder about with the tests we can maybe tackle there, e.g., I don't see a need to do || 0 for the new fields as they should always be defined in a compliant implementation.)

I also left a couple remarks inline.

annevk · 2019-06-14T13:12:20Z

storage.bs


+The <dfn export>application cache site storage usage</dfn> for an <a for=/>origin</a>
+<var>origin</var> is a rough estimate of the amount of bytes used in <a>Application Cache</a>
+in <var>origin</var>'s <a>site storage unit</a>. [[!HTML]]


This one can contain opaque responses right? Given that this is a new place where we expose that information, I think we should also detail how to address the issue. Same for caches below.

I intentionally pulled the security part out into a separate PR in which I address the opaque response + padding issues. If you believe they should be merged in the PR, I can consolidate the two. Thoughts?

The other PR did not have the necessary wording either, right? But yeah, I think they should be included here and should probably be noted closer to where we define these features. Not in a separate section somewhere. The separate section seems like a fine place to highlight some of the trickier areas, but in general we should note security requirements right alongside general implementation requirements.

I've taken a stab at adding a recommendation for padding. Please take a look when you get a chance, thanks!

annevk · 2019-06-14T13:12:51Z

storage.bs

+in <var>origin</var>'s <a>site storage unit</a>. [[!HTML]]

+The <dfn export>caches site storage usage</dfn> for an <a for=/>origin</a>
+<var>origin</var> is a rough estimate of the amount of bytes used in <a attribute>caches</a> API


This should be something like {{Caches}} or {{Cache}}, no? Referring to the getter seems a little weird.

My understanding was that it should link here: https://w3c.github.io/ServiceWorker/#cache-objects

When I used {{Cache}}, it linked here https://w3c.github.io/ServiceWorker/#cache.

When I used {{Caches}} or {{cache-objects}}, it did not create a link (it was just grey text).

Please advise.

I guess you want {{CacheStorage}}? @jakearchibald thoughts?

The other definitions don't link to an interface... I'd expect a link to the top level concept rather than an interface, e.g. https://w3c.github.io/ServiceWorker/#cache-objects

I don't think there's an export from SW there. So yeah, @jakearchibald ?

Note that we could do this by adding a <pre class=anchors>...</pre> section w/o an explicit export from SW, but let's figure out what we want here. FWIW over in https://wicg.github.io/web-locks/ I wanted to do something similar and ended up with "Caches [Service-Workers])" where "Caches" was not a link and latter bit was just a biblio link, assuming readers would figure it out.

jarryd999 · 2019-07-12T22:48:49Z

Thanks for the review!

Are other implementers on board with exposing this?

There has been no public support, but also no opposition.

Are there bugs filed against those implementations?

Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1565716
Safari: https://bugs.webkit.org/show_bug.cgi?id=199762

Is there a PR to make the tests stop being tentative that we can land together with this assuming all is in order? (There's a couple minor things I wonder about with the tests we can maybe tackle there, e.g., I don't see a need to do || 0 for the new fields as they should always be defined in a compliant implementation.)

https://chromium-review.googlesource.com/c/chromium/src/+/1700564

squash 47187f6 Address style nits from domenic. squash 0fd3b44 Explicitly define each storage system's usage.

To be landed when the spec changes land. whatwg/storage#69 Bug:904000 Change-Id: I630c3b73b4ad52f901eabddc7902973309a655f9

annevk · 2020-05-04T10:24:30Z

@jarryd999 could you check if #86 would make it significantly easier to define this? It seems to me we'd only have to define storage usage of a bucket's area and this would all fall out automatically.

Is there a reason for excluding localStorage?

jarryd999 · 2020-05-06T19:09:03Z

@annevk I'm not seeing how #86 makes this significantly easier to define. I think we are partially aligned: I've always imagined usageDetails to show a breakdown of usage per-bucket. However, I am confused why the addition of buckets would mean this "would all fall out automatically", as I think the only change would be to the scope of usageDetails. Could you help me understand?

localStorage was excluded because it is currently not quota-managed. My thoughts on this are that all storage should be quota-managed, or that at the least, quota should be aware of all storage, and thus it should be displayed in usageDetails. However, I'm not too familiar with the story around localStorage.

annevk · 2020-05-07T04:13:55Z

Various APIs fit into a single bucket, each assigned to an area. For this we'd only need to define the primitive of obtaining the size of an area and then with some loops we can fill in all the details.

jarryd999 force-pushed the usageDetails branch from dff574e to 1e7028a Compare April 22, 2019 23:27

domenic reviewed Apr 30, 2019

View reviewed changes

storage.bs Outdated Show resolved Hide resolved

storage.bs Outdated Show resolved Hide resolved

storage.bs Outdated Show resolved Hide resolved

storage.bs Outdated Show resolved Hide resolved

jarryd999 force-pushed the usageDetails branch 3 times, most recently from 56dc8be to 07ebfe8 Compare May 6, 2019 16:01

jarryd999 mentioned this pull request May 6, 2019

Storage Quota Usage Details w3ctag/design-reviews#365

Closed

5 tasks

domenic reviewed May 31, 2019

View reviewed changes

storage.bs Outdated Show resolved Hide resolved

storage.bs Outdated Show resolved Hide resolved

storage.bs Outdated Show resolved Hide resolved

storage.bs Outdated Show resolved Hide resolved

storage.bs Outdated Show resolved Hide resolved

storage.bs Outdated Show resolved Hide resolved

jarryd999 force-pushed the usageDetails branch 2 times, most recently from ecceb0a to 657ce68 Compare June 3, 2019 22:39

domenic approved these changes Jun 10, 2019

View reviewed changes

jarryd999 mentioned this pull request Jun 11, 2019

StorageEstimate.usageDetails mdn/sprints#1702

Closed

2 tasks

annevk reviewed Jun 14, 2019

View reviewed changes

Add UsageDetails dictionary.

f74aea3

squash 47187f6 Address style nits from domenic. squash 0fd3b44 Explicitly define each storage system's usage.

jarryd999 force-pushed the usageDetails branch from 657ce68 to f74aea3 Compare July 17, 2019 22:33

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 4, 2019

Quota: Make usageDetails WPT stop being tentative.

124085d

To be landed when the spec changes land. whatwg/storage#69 Bug:904000 Change-Id: I630c3b73b4ad52f901eabddc7902973309a655f9

chromium-wpt-export-bot mentioned this pull request Oct 4, 2019

Quota: Make usageDetails WPT stop being tentative. web-platform-tests/wpt#19520

Closed

lekoaf mentioned this pull request May 27, 2020

feat: StorageEstimateUsageDetails microsoft/TypeScript-DOM-lib-generator#859

Closed

annevk mentioned this pull request May 29, 2020

Moving text from HTML's web storage into the Storage Standard #95

Open

Base automatically changed from master to main January 15, 2021 07:41

mkruisselbrink mentioned this pull request Oct 24, 2022

StorageManager.estimate web-platform-tests/interop#221

Closed

Conversation

jarryd999 commented Apr 22, 2019 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jarryd999 commented Jun 4, 2019

Uh oh!

domenic left a comment

Choose a reason for hiding this comment

Uh oh!

inexorabletash commented Jun 10, 2019

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jarryd999 commented Jul 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

annevk commented May 4, 2020

Uh oh!

jarryd999 commented May 6, 2020

Uh oh!

annevk commented May 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

jarryd999 commented Apr 22, 2019 •

edited by pr-preview bot

Loading

jarryd999 commented Jul 12, 2019 •

edited

Loading